-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
Adds OpenJS Footer #8577
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Adds OpenJS Footer #8577
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
👋 Codeowner Review RequestThe following codeowners have been identified for the changed files: Team reviewers: @nodejs/nodejs-website @nodejs/web-infra Please review the changes when you have a chance. Thank you! 🙏 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8577 +/- ##
==========================================
+ Coverage 74.95% 74.99% +0.03%
==========================================
Files 103 103
Lines 9037 9062 +25
Branches 312 313 +1
==========================================
+ Hits 6774 6796 +22
- Misses 2261 2264 +3
Partials 2 2 ☔ View full report in Codecov by Sentry. |
📦 Build Size ComparisonSummary
Changes➕ Added Assets (2)
➖ Removed Assets (2)
|
| "trademarkPolicy": "Trademark Policy", | ||
| "trademarkList": "Trademark List", | ||
| "cookiePolicy": "Cookie Policy", | ||
| "security": "Security Policy" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The retention of the security policy here means it is ADDED to the OpenJS footer. I think this is acceptable, as we purposely put it there at one point to expand visibility.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file is the only place the new writing is placed. It's slightly duplicative between legal paragraph and the links which are used later, but I think it's acceptable considering alternatives
| "containers": { | ||
| "footer": { | ||
| "legal": "Copyright <openjsf>OpenJS Foundation</openjsf> and Node.js contributors. All rights reserved. The <openjsf>OpenJS Foundation</openjsf> has registered trademarks and uses trademarks. For a list of trademarks of the <openjsf>OpenJS Foundation</openjsf>, please see our <trademarkPolicy>Trademark Policy</trademarkPolicy> and <trademarkList>Trademark List</trademarkList>. Trademarks and logos not indicated on the <trademarkList>list of OpenJS Foundation trademarks</trademarkList> are trademarks™ or registered® trademarks of their respective holders. Use of them does not imply any affiliation with or endorsement by them.", | ||
| "links": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why we need these individual keys?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The links? The diff is hard to see if you are point at line 5 or line 6.
The openjs guidance is a paragraph with links and a list of links afterwards, which makes for duplication...somewhere. I've chosen to put the duplication in the translation strings rather than the code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR implements the OpenJS Foundation best practices for website footers by restructuring the footer into three sections: a primary section with version badges, a secondary section with social media links, and a new legal section containing copyright text and policy links.
Changes:
- Restructured Footer component to support a new 'legal' slot alongside existing 'primary' and 'secondary' slots
- Added new WithLegal component to render OpenJS Foundation copyright text with embedded links and a list of policy links
- Updated footer navigation configuration with additional OpenJS Foundation links (Terms of Use, Bylaws, Trademark List, Cookie Policy)
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/ui-components/src/Containers/Footer/index.tsx | Restructured footer layout into two rows, added legal slot support |
| packages/ui-components/src/Containers/Footer/index.module.css | Added styling for new row-based layout and legal section with responsive design |
| apps/site/components/withLegal.tsx | New component to render legal paragraph with rich text links and footer links list |
| apps/site/components/withFooter.tsx | Integrated WithLegal component into footer legal slot |
| packages/i18n/src/locales/en.json | Added legal paragraph text and new footer link translations |
| apps/site/navigation.json | Updated footer links with new OpenJS Foundation policy links |
| pnpm-workspace.yaml | Added @nodejs/doc-kit to onlyBuiltDependencies list |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
7bb974b to
aa4f93e
Compare
kj-powell
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Thanks!

Description
Expands the footer to now have three main sections.
Our top-bar that has version info on the left and social info on the right
A bottom section devoted to legal information
Validation
https://nodejs-org-git-openjs-footer-openjs.vercel.app/en
Related Issues
closes #8574
Check List
pnpm formatto ensure the code follows the style guide.pnpm testto check if all tests are passing.pnpm buildto check if the website builds without errors.